Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add chainsaw based e2e tests #506

Merged
merged 38 commits into from
Nov 26, 2024
Merged

feat: add chainsaw based e2e tests #506

merged 38 commits into from
Nov 26, 2024

Conversation

eddycharly
Copy link
Contributor

What

This PR adds chainsaw based e2e tests (https://github.com/kyverno/chainsaw)

How

Added a couple of chainsaw tests and updated ci to run those tests.

Breaking Changes

NONE

@github-actions github-actions bot added the area/ci Issues/PRs relating to CI label Nov 14, 2024
@eddycharly
Copy link
Contributor Author

Talked about that with someone at KubeCon today, let's discuss further tomorrow =)

@jonstacks
Copy link
Collaborator

@eddycharly, thank you for stopping by the booth. I still need to talk with the rest of our team, but I think we will end up taking this and gradually build up more chainsaw tests.

Just linking the product page for others on our team as I found it helpful: https://kyverno.github.io/chainsaw/latest/

@eddycharly
Copy link
Contributor Author

Thanks @jonstacks take your time and i hope you will find chainsaw useful.
BTW the nix package is here.

@eddycharly eddycharly marked this pull request as ready for review November 15, 2024 06:15
@eddycharly eddycharly requested a review from a team as a code owner November 15, 2024 06:15
@hjkatz
Copy link
Contributor

hjkatz commented Nov 15, 2024

Wow thank you @eddycharly for this addition! I'll be looking to try and use these tests today and see if we can get this merged.

e2e tests like these will really help us reach stability 😄

Copy link
Contributor

hjkatz commented Nov 15, 2024

@eddycharly I would like to play around with this locally and submit some changes for nix. Though I'm not exactly sure how to collaborate on this branch... perhaps I create my own fork and submit PRs to your fork so we can update this PR for our main fork? 😂 seems like a lot but maybe that would work?

Thankfully it seems Github has thought of this: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork#enabling-repository-maintainer-permissions-on-existing-pull-requests

Would you kindly enable this setting so we could collaborate?

Makefile Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@eddycharly
Copy link
Contributor Author

Would you kindly enable this setting so we could collaborate?

@hjkatz i'll come to the booth with my laptop, will be easier ;)

Copy link
Contributor

hjkatz commented Nov 15, 2024

I'm unfortunately not at kubecon - But I'm sure @jonstacks will love to help out.

FWIW I think you did have this setting enabled because I was able to push a commit to your fork's branch: https://github.com/ngrok/ngrok-operator/pull/506/commits (2nd commit at the bottom)

Copy link
Contributor

@Alice-Lilith Alice-Lilith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh... yaml driven k8s tests. This is neat 👀

@eddycharly
Copy link
Contributor Author

I think you did have this setting enabled because I was able to push a commit to your fork's branch

Ok, cool. I'll step by the booth anyway. Let me know if you have more questions.

@eddycharly
Copy link
Contributor Author

Hi folks, i'm back at work. Let me know if you consider merging this PR and use chainsaw, i can help with changes if you want.
cc @jonstacks @hjkatz @Alice-Lilith

@hjkatz
Copy link
Contributor

hjkatz commented Nov 22, 2024

@eddycharly Sorry for the delay - We're are excited to merge this and I'll be focused on it today.

Q. Do you have any additions for the PR before we merge?

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@eddycharly
Copy link
Contributor Author

Do you have any additions for the PR before we merge?

Not really, next step would be to add more tests, maybe i will check the issues to see if they can be converted to a test easily.
In Kyverno user reporting issues usually contain manifests to reproduce the issue and it's often very easy to turn those into a test, maybe it's the same here 🤞

@eddycharly
Copy link
Contributor Author

BTW we have a dedicated slack channel you can use if you have questions https://kubernetes.slack.com/archives/C067LUFL43U

@eddycharly
Copy link
Contributor Author

@hjkatz either the pod doesn't start or keeps restarting because of invalid api-key/token 🤔
I can look at it later if it helps, you can also temporarily disable this test until you come up with a solution for the api-key/token.

Copy link
Contributor

hjkatz commented Nov 22, 2024

hah, sorry for any emails - We're iterating on this right now trying to figure out exactly how we want these to run.

@hjkatz
Copy link
Contributor

hjkatz commented Nov 26, 2024

@eddycharly Sorry for all of the crazy noise! This is finally ready to merge. There were some adjustments @jonstacks and I decided to make so the e2e tests will work for both ngrok-op/main and forks. tl; dr:

  • Separate out changes and build-and-test into composite github actions
  • Create 2 workflows: ci.yaml (pull_request) and test.yaml (pull_request_target)
  • Ensure that each workflow has access to secrets (or not) as needed

@hjkatz hjkatz merged commit 984ca2a into ngrok:main Nov 26, 2024
8 checks passed
@eddycharly eddycharly deleted the chainsaw branch November 26, 2024 19:18
@eddycharly
Copy link
Contributor Author

eddycharly commented Nov 26, 2024

@hjkatz @jonstacks that's awesome !

I hope you like chainsaw, don't hesitate to ask questions on our slack channel if needed or file issues or contribute to the project if you want.

Additionally you can add yourself in the ADOPTERS file if you want.

I didn't have much time to help but i will try to search the issues and see what tests it makes sense to add when i have more time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Issues/PRs relating to CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants